-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Fix flaky test: test_mkldnn.test_activation #12377 #12418
Conversation
Please enable the case which has been skipped in another PR. |
@marcoabreu @lebeg please help take a reveiw |
@@ -292,7 +291,7 @@ def check_activation_training(stype): | |||
in_location = [mx.nd.array(data_tmp).tostype(stype)] | |||
|
|||
test = mx.symbol.Activation(data, act_type="relu") | |||
check_numeric_gradient(test, in_location, numeric_eps=1e-2, rtol=0.16, atol=1e-4) | |||
check_numeric_gradient(test, in_location, numeric_eps=1e-6, rtol=0.16, atol=1e-4) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, this change is lowering the epsilon by 4 magnitudes. If this test was flaky before due to computational precision wouldn't this change make it worse?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, it will improve the precision of the reference results because we use the smaller eps for the finite difference method.
Previously, the flaky test is caused by the large eps which can't simulate the small difference.
The numerical calculation sometimes is a little tricky :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This epsilon would have an impact on the baseline calculation instead of mkldnn calculation. And the smaller the epsilon is, the more accurate the baseline (gradient referring to theano) is. So this change won't make it worse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pengzhao-intel @luobao-intel thanks for your explanations!
Thanks for the fix @luobao-intel ! |
…che#12418)" This reverts commit 445967e.
* test_activation_rec_eps * enable case
* Revert "Removing the re-size for validation data, which breaking the validation accuracy of CIFAR training (#12362)" This reverts commit ceabcaa. * Revert "[MXNET-580] Add SN-GAN example (#12419)" This reverts commit 46a5cee. * Revert "Remove regression checks for website links (#12507)" This reverts commit 619bc3e. * Revert "Revert "Fix flaky test: test_mkldnn.test_activation #12377 (#12418)" (#12516)" This reverts commit 7ea0533. * Revert "further bump up tolerance for sparse dot (#12527)" This reverts commit 90599e1. * Revert "Fix broken URLs (#12508)" This reverts commit 3d83c89. * Revert "Temporarily disable flaky tests (#12520)" This reverts commit 35ca13c. * Revert "Add support for more req patterns for bilinear sampler backward (#12386)" This reverts commit 4ee866f. * Revert "Change the way NDArrayIter handle the last batch (#12285)" This reverts commit 597a637.
* test_activation_rec_eps * enable case
…che#12418)" (apache#12516) This reverts commit 445967e.
* Revert "Removing the re-size for validation data, which breaking the validation accuracy of CIFAR training (apache#12362)" This reverts commit ceabcaa. * Revert "[MXNET-580] Add SN-GAN example (apache#12419)" This reverts commit 46a5cee. * Revert "Remove regression checks for website links (apache#12507)" This reverts commit 619bc3e. * Revert "Revert "Fix flaky test: test_mkldnn.test_activation apache#12377 (apache#12418)" (apache#12516)" This reverts commit 7ea0533. * Revert "further bump up tolerance for sparse dot (apache#12527)" This reverts commit 90599e1. * Revert "Fix broken URLs (apache#12508)" This reverts commit 3d83c89. * Revert "Temporarily disable flaky tests (apache#12520)" This reverts commit 35ca13c. * Revert "Add support for more req patterns for bilinear sampler backward (apache#12386)" This reverts commit 4ee866f. * Revert "Change the way NDArrayIter handle the last batch (apache#12285)" This reverts commit 597a637.
Description
Fix the flaky test failure : test_mkldnn.test_activation. #12377
The problem locates in the finite difference method for gradient comparison. In this case, the large eps caused the wrong calculation and this pull request reduces the eps for more accurate calculation.
@pengzhao-intel